Textmate cooperation #4397

georgewfraser · 2020-05-09T20:07:26Z

This PR tweaks the fallback TextMate scopes to make them more consistent with the existing grammar and other languages, and edits the builtin TextMate grammar to align with semantic coloring. Before is on the left, after is on the right:

Use keyword.other for regular keywords instead of keyword. This is a really peculiar quirk of TextMate conventions, but virtually all TextMate grammars use keyword.other (colored blue in VSCode Dark+) for regular keywords and keyword.control (colored purple in VSCode Dark+) for control keywords. The TextMate scope keyword is colored like control keywords, not regular keywords. It may seem strange that the keyword scope is not the right fallback for the keyword semantic token, but TextMate has a long and weird history. Note how keywords change from purple back to blue (what they were before semantic coloring was added):

(1) Use punctuation.section.embedded for format specifiers. This aligns with how Typescript colors formatting directives:

(2) Consistently use entity.name.type.* scopes for type names. Avoid using entity.name.* which gets colored like a keyword.

(3) Use Property instead of Member for fields. Property and Member are very similar, but if you look at the TextMate fallback scopes, it's clear that Member is intended for function-like-things (methods?) and Property is intended for variable-like-things.

(4) Color for as a regular keyword when it's part of impl Trait for Struct.

(5) Use variable.other.constant for constants instead of entity.name.constant. In the latest VSCode insiders, variable.other.constant has a subtly different color that differentiates constants from ordinary variables. It looks close to the green of types but it's not the same---it's a new color recently added to take advantage of semantic coloring.

I also made some minor changes that make the TextMate scopes better match the semantic scopes. The effect of this for the user is you observe less of a change when semantic coloring "activates". You can see the changes I made relative to the built-in TextMate grammar here:

https://github.com/rust-analyzer/rust-analyzer/pull/4397/files/a91d15c80c337dd1afb0eddd5eb048010d098ac7..97428b6d52d25f810dbd7d7a8d787740c58bfbd2#diff-6966c729b862f79f79bf7258eb3e0885

aloucks · 2020-05-10T03:39:38Z

The change to lifetime coloring is the first screenshot is worse. I definitely prefer it being different from types.

bjorn3 · 2020-05-10T08:45:48Z

editors/code/rust.tmGrammar.json

@@ -600,7 +600,7 @@
 		},
 		"core_types": {
 			"comment": "Built-in/core type",
-			"name": "storage.type.core.rust",
+			"name": "entity.name.type.core.rust",


This will mean that builtin types no longer get any different syntax highlighting compared to other types, right?

Yes that’s right. I’m open to changing these choices—the main goal of this PR is just to make the TextMate grammar and the semantic colors consistent with each other, and to fix anything that’s clearly just a mistake/accident inherited from TextMate.

georgewfraser · 2020-05-10T18:35:57Z

I've discovered a few more subtle disagreements between TextMate and semantic coloring and fixed them:

as should be colored as a keyword (fixed TextMate)
in should be colored as a control keyword (fixed semantic coloring)
for should be colored as a regular keyword when it's part of impl Trait for Struct (fixed semantic coloring)

…Mate grammar and other languages

georgewfraser · 2020-05-10T20:15:32Z

(Rebased)

…between TextMate and semantic

georgewfraser · 2020-05-10T20:38:46Z

@aloucks Since changing lifetimes to look like types seems to be controversial, I undid that to keep this PR narrowly focused on harmonizing TextMate and semantic coloring.

@bjorn3 right now, TextMate is coloring builtins like keywords, semantic coloring is coloring them like types. Personally I think coloring them like types is better, but I'm happy to go with the consensus either way---we should just be consistent between TextMate and semantic coloring.

georgewfraser · 2020-05-10T21:03:45Z

(Edited the description to show before-and-after more clearly)

aloucks · 2020-05-10T21:53:39Z

I think & looks weird now that it's the same color as self (and presumable mut as well). I think & looks better uncolored/grey.

georgewfraser · 2020-05-10T22:40:24Z

@aloucks Yeah I suppose that's a change as well. It does match the way rust is colored elsewhere, but in the spirit of "let's keep this PR focused on harmonization not change" I put that back.

georgewfraser · 2020-05-10T22:43:30Z

Use punctuation.section.embedded for format specifiers is also technically a change, though I think it's such a no-brainer that I'm leaving it in for now. If anyone disagrees, I'm happy to take it out and make that a separate PR :)

DJMcNab · 2020-05-11T11:36:44Z

It might be worth using member for fields which have function types or have Fn[Once/Mut]() bounds.
I think this matches the behaviour of the typescript server. Otherwise the described changes look excellent to me, although I haven't tested them

matklad

bors r+

I am still personally a huge anti-fan of having tmGrammar for Rust at all, (here's the grammar I use), but this is clearly a huge improvement in practice! Thanks @georgewfraser!

matklad · 2020-05-11T17:18:54Z

crates/ra_ide/src/syntax_highlighting.rs

-                | T![while] => h | HighlightModifier::ControlFlow,
+                | T![while]
+                | T![in] => h | HighlightModifier::ControlFlow,
+                T![for] if !is_child_of_impl(element) => h | HighlightModifier::ControlFlow,


bors · 2020-05-11T17:41:40Z

Build succeeded:

4436: Use .rust suffix on scopes r=matklad a=georgewfraser This PR should have no effect on people using any of the default themes, but it is possible there are people with custom themes that rely on the .rust suffix on textmate scopes, which I neglected to use consistently in #4397. Co-authored-by: George Fraser <[email protected]>

4400: Enhanced coloring r=georgewfraser a=georgewfraser This PR builds on #4397 to enhance the existing syntax coloring. ## Underline mutable variables The textmate scope `markup.underline` underlines identifiers, which is a nice way to make mutable vars stand out: <img width="327" alt="Screen Shot 2020-05-09 at 1 18 55 PM" src="https://user-images.githubusercontent.com/1369240/81484179-8bb47d80-91f8-11ea-997d-1dcffbe44aa7.png"> ## Italicize static variables The textmate scope `markup.italic` italicizes identifiers. Italic = static is a common convention in IDEs like IntelliJ: <img width="288" alt="Screen Shot 2020-05-09 at 1 19 14 PM" src="https://user-images.githubusercontent.com/1369240/81484236-cd452880-91f8-11ea-8478-505ee49bc8b3.png"> Co-authored-by: George Fraser <[email protected]>

This was referenced May 9, 2020

Optimize syntax coloring #4383

Closed

Enhanced coloring #4400

Merged

bjorn3 reviewed May 10, 2020

View reviewed changes

georgewfraser added 10 commits May 10, 2020 13:04

Tweak the textmate fallback scopes to be consistent with builtin Text…

da0c8d9

…Mate grammar and other languages

Use Property instead of Member for fields

cff0d6b

Import built-in textmate grammar, with no changes

a91d15c

Color lifetimes like types

d5d154b

Color sigil like keyword

0881c88

Color core types as types, not keywords

60e8e56

No longer true

e86379a

Color as as a keyword

46566d7

Color in as a control keyword

3f1c736

Color for as a regular keyword when it's part of impl _ for _

63b75a4

georgewfraser force-pushed the textmate_cooperation branch from 257c2ab to 63b75a4 Compare May 10, 2020 20:15

Change lifetimes back to keyword-ish, tweak builtins for consistency …

2d2b32a

…between TextMate and semantic

Put sigil back to keyword.operator

97428b6

matklad reviewed May 11, 2020

View reviewed changes

bors bot merged commit 848aa56 into rust-lang:master May 11, 2020

matklad mentioned this pull request May 11, 2020

Semantic Highlighting: Methods should be methods, fields should be fields #4399

Closed

georgewfraser mentioned this pull request May 12, 2020

Proposal: color attributes using regular syntax coloring #4430

Closed

georgewfraser mentioned this pull request May 12, 2020

Use .rust suffix on scopes #4436

Merged

bjorn3 mentioned this pull request Sep 21, 2020

Incorrect string syntax highlighting in attributes in VS Code #6042

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Textmate cooperation #4397

Textmate cooperation #4397

georgewfraser commented May 9, 2020 •

edited

Loading

aloucks commented May 10, 2020

bjorn3 May 10, 2020

georgewfraser May 10, 2020

georgewfraser commented May 10, 2020 •

edited

Loading

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

aloucks commented May 10, 2020

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

DJMcNab commented May 11, 2020

matklad left a comment

matklad May 11, 2020

bors bot commented May 11, 2020

Textmate cooperation #4397

Textmate cooperation #4397

Conversation

georgewfraser commented May 9, 2020 • edited Loading

aloucks commented May 10, 2020

bjorn3 May 10, 2020

Choose a reason for hiding this comment

georgewfraser May 10, 2020

Choose a reason for hiding this comment

georgewfraser commented May 10, 2020 • edited Loading

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

aloucks commented May 10, 2020

georgewfraser commented May 10, 2020

georgewfraser commented May 10, 2020

DJMcNab commented May 11, 2020

matklad left a comment

Choose a reason for hiding this comment

matklad May 11, 2020

Choose a reason for hiding this comment

bors bot commented May 11, 2020

georgewfraser commented May 9, 2020 •

edited

Loading

georgewfraser commented May 10, 2020 •

edited

Loading